Implement post /tasks/{task id}/labels/ api to add a label to a task #43
Conversation
Summary by CodeRabbit
WalkthroughA new API endpoint for adding a label to a task has been implemented. The update introduces a serializer for input validation, a service method for business logic, a view method for request handling, a URL route, and corresponding documentation in the README. No existing code was modified or removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API (TaskView.post_label)
participant AddLabelSerializer
participant TaskService
participant TaskRepo
participant LabelRepo
Client->>API (TaskView.post_label): POST /tasks/{task_id}/labels {label_id}
API->>AddLabelSerializer: Validate request data
AddLabelSerializer-->>API: Validation result
API->>TaskService: add_label_to_task(task_id, label_id)
TaskService->>TaskRepo: get_task_by_id(task_id)
TaskRepo-->>TaskService: Task object or None
TaskService->>LabelRepo: get_label_by_id(label_id)
LabelRepo-->>TaskService: Label object or None
TaskService->>TaskRepo: update task with new label (if valid)
TaskRepo-->>TaskService: Update result
TaskService-->>API: None or error
API-->>Client: 200 OK / error response
Possibly related issues
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)todo/serializers/add_label_serializer.py(1 hunks)todo/services/task_service.py(1 hunks)todo/urls.py(1 hunks)todo/views/task.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
todo/urls.py (1)
todo/views/task.py (1)
TaskView(17-130)
todo/serializers/add_label_serializer.py (1)
todo/models/common/pyobjectid.py (1)
PyObjectId(4-15)
🪛 markdownlint-cli2 (0.17.2)
README.md
102-102: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
103-103: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
103-103: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
124-124: Files should end with a single newline character
null
(MD047, single-trailing-newline)
🪛 Ruff (0.11.9)
todo/services/task_service.py
226-226: Undefined name PyObjectId
(F821)
226-226: Undefined name PyObjectId
(F821)
🔇 Additional comments (5)
todo/urls.py (1)
8-8: LGTM! Well-structured URL pattern.The URL pattern follows REST conventions and correctly maps POST requests to the
post_labelmethod. The route structure/tasks/{task_id}/labelsis intuitive and follows the resource hierarchy.todo/views/task.py (4)
9-9: LGTM! New imports follow existing patterns.The new imports are correctly placed and follow the existing import organization in the file.
Also applies to: 15-15
87-89: LGTM! Serializer validation follows established patterns.The serializer validation logic correctly follows the same pattern as the existing
postmethod, ensuring consistency across the codebase.
64-108:✅ Verification successful
Verify the AddLabelSerializer dependency exists.
The method implementation looks solid and follows the established patterns in the codebase. Let's verify that the
AddLabelSerializerexists and handles the expectedlabel_idfield correctly.
🏁 Script executed:
#!/bin/bash # Description: Verify AddLabelSerializer exists and check its field definitions # Expected: Find the serializer class with label_id field ast-grep --pattern $'class AddLabelSerializer { $$$ }' # Check the serializer file content cat todo/serializers/add_label_serializer.py # Verify the label_id field usage in the serializer rg -A 5 -B 5 "label_id" todo/serializers/add_label_serializer.pyLength of output: 942
AddLabelSerializer Verified
I’ve confirmed that
AddLabelSerializeris defined intodo/serializers/add_label_serializer.py, declares a requiredlabel_idfield, and correctly validates it by converting toPyObjectId. No further changes are needed here—this implementation aligns with our established patterns.
91-107:✅ Verification successful
Verify the service method exists and has correct signature.
The error handling pattern correctly follows the established conventions in the codebase. However, let's verify that the
TaskService.add_label_to_taskmethod exists and accepts the expected parameters.
🏁 Script executed:
#!/bin/bash # Description: Verify TaskService.add_label_to_task method exists and check its signature # Expected: Find the method definition with task_id and label_id parameters ast-grep --pattern $'class TaskService { $$$ def add_label_to_task($$$) { $$$ } $$$ }' # Also check for any static method decorator rg -A 10 "def add_label_to_task"Length of output: 777
TaskService.add_label_to_task signature verified
The method
add_label_to_task(task_id: PyObjectId, label_id: PyObjectId) -> Noneexists intodo/services/task_service.pyand matches the expected signature. No further changes required.
| from todo.repositories.task_repository import TaskRepository | ||
| from todo.repositories.label_repository import LabelRepository |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move imports to module level.
Imports inside functions are generally an anti-pattern and can hurt performance. These repositories are already imported at the module level.
Remove these local imports since TaskRepository and LabelRepository are already imported at lines 17-18:
- from todo.repositories.task_repository import TaskRepository
- from todo.repositories.label_repository import LabelRepository
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from todo.repositories.task_repository import TaskRepository | |
| from todo.repositories.label_repository import LabelRepository |
🤖 Prompt for AI Agents
In todo/services/task_service.py around lines 237 to 238, remove the local
imports of TaskRepository and LabelRepository inside the function since they are
already imported at the module level at lines 17-18. This will avoid redundant
imports and improve performance.
| ) | ||
|
|
||
| @staticmethod | ||
| def add_label_to_task(task_id: PyObjectId, label_id: PyObjectId) -> None: |
There was a problem hiding this comment.
Fix the missing import for PyObjectId.
The static analysis correctly identifies that PyObjectId is not imported. Add the import at the module level with the other imports.
Add this import at the top of the file with the other imports:
+from todo.models.common.pyobjectid import PyObjectId📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def add_label_to_task(task_id: PyObjectId, label_id: PyObjectId) -> None: | |
| +from todo.models.common.pyobjectid import PyObjectId |
🧰 Tools
🪛 Ruff (0.11.9)
226-226: Undefined name PyObjectId
(F821)
226-226: Undefined name PyObjectId
(F821)
🤖 Prompt for AI Agents
In todo/services/task_service.py at line 226, the function uses PyObjectId but
it is not imported. Add the import statement for PyObjectId at the top of the
file alongside the other imports to resolve the missing import error.
| raise ValueError(ApiErrorResponse( | ||
| statusCode=404, | ||
| message="Task not found", | ||
| errors=[{"detail": f"Task with ID {task_id} does not exist"}] | ||
| )) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistency with existing error response patterns.
The error response structure here doesn't match the pattern used elsewhere in this file. The existing pattern uses ApiErrorDetail objects with proper structure.
Update the error response to match the existing pattern:
- raise ValueError(ApiErrorResponse(
- statusCode=404,
- message="Task not found",
- errors=[{"detail": f"Task with ID {task_id} does not exist"}]
- ))
+ raise ValueError(
+ ApiErrorResponse(
+ statusCode=404,
+ message="Task not found",
+ errors=[
+ ApiErrorDetail(
+ source={ApiErrorSource.PARAMETER: "task_id"},
+ title="Task not found",
+ detail=f"Task with ID {task_id} does not exist",
+ )
+ ],
+ )
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise ValueError(ApiErrorResponse( | |
| statusCode=404, | |
| message="Task not found", | |
| errors=[{"detail": f"Task with ID {task_id} does not exist"}] | |
| )) | |
| raise ValueError( | |
| ApiErrorResponse( | |
| statusCode=404, | |
| message="Task not found", | |
| errors=[ | |
| ApiErrorDetail( | |
| source={ApiErrorSource.PARAMETER: "task_id"}, | |
| title="Task not found", | |
| detail=f"Task with ID {task_id} does not exist", | |
| ) | |
| ], | |
| ) | |
| ) |
🤖 Prompt for AI Agents
In todo/services/task_service.py around lines 243 to 247, the raised ValueError
uses an ApiErrorResponse with a list of dictionaries for errors, which is
inconsistent with the rest of the file that uses ApiErrorDetail objects. To fix
this, replace the dictionary in the errors list with an instance of
ApiErrorDetail, properly initialized with the error detail message, ensuring the
error response structure matches the existing pattern.
| if label_id not in task.labels: | ||
| task.labels.append(label_id) | ||
| TaskRepository.update(task) |
There was a problem hiding this comment.
Convert PyObjectId to string before storing in labels list.
The task.labels list should contain string IDs, not PyObjectId objects. Looking at the existing code patterns in this file (e.g., line 158), label IDs are stored as strings.
Convert the label_id to string before adding:
- if label_id not in task.labels:
- task.labels.append(label_id)
+ label_id_str = str(label_id)
+ if label_id_str not in task.labels:
+ task.labels.append(label_id_str)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if label_id not in task.labels: | |
| task.labels.append(label_id) | |
| TaskRepository.update(task) | |
| label_id_str = str(label_id) | |
| if label_id_str not in task.labels: | |
| task.labels.append(label_id_str) | |
| TaskRepository.update(task) |
🤖 Prompt for AI Agents
In todo/services/task_service.py around lines 263 to 265, the code appends
label_id directly to task.labels, but task.labels should store string IDs, not
PyObjectId objects. Fix this by converting label_id to a string before appending
it to task.labels, ensuring consistency with existing code patterns in the file.
| raise ValueError(ApiErrorResponse( | ||
| statusCode=404, | ||
| message="Label not found", | ||
| errors=[{"detail": f"Label with ID {label_id} does not exist"}] | ||
| )) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistency with existing error response patterns.
Same issue as above - the error response structure should match the existing pattern.
Update the error response to match the existing pattern:
- raise ValueError(ApiErrorResponse(
- statusCode=404,
- message="Label not found",
- errors=[{"detail": f"Label with ID {label_id} does not exist"}]
- ))
+ raise ValueError(
+ ApiErrorResponse(
+ statusCode=404,
+ message="Label not found",
+ errors=[
+ ApiErrorDetail(
+ source={ApiErrorSource.PARAMETER: "label_id"},
+ title="Label not found",
+ detail=f"Label with ID {label_id} does not exist",
+ )
+ ],
+ )
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise ValueError(ApiErrorResponse( | |
| statusCode=404, | |
| message="Label not found", | |
| errors=[{"detail": f"Label with ID {label_id} does not exist"}] | |
| )) | |
| raise ValueError( | |
| ApiErrorResponse( | |
| statusCode=404, | |
| message="Label not found", | |
| errors=[ | |
| ApiErrorDetail( | |
| source={ApiErrorSource.PARAMETER: "label_id"}, | |
| title="Label not found", | |
| detail=f"Label with ID {label_id} does not exist", | |
| ) | |
| ], | |
| ) | |
| ) |
🤖 Prompt for AI Agents
In todo/services/task_service.py around lines 252 to 256, the raised
ValueError's ApiErrorResponse structure does not match the existing error
response pattern. Modify the ApiErrorResponse to follow the consistent format
used elsewhere in the codebase, ensuring the keys and structure align with the
standard error response pattern, such as using 'status_code' instead of
'statusCode' and adjusting the message and errors fields accordingly.
| def validate_label_id(self, value): | ||
| try: | ||
| return PyObjectId(value) | ||
| except Exception: | ||
| raise serializers.ValidationError("Invalid label ID format") No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve exception handling specificity.
The generic except Exception could mask unexpected errors. Based on the PyObjectId implementation in the relevant code snippets, it raises ValueError for invalid ObjectIds.
Use more specific exception handling:
def validate_label_id(self, value):
try:
return PyObjectId(value)
- except Exception:
+ except (ValueError, TypeError):
raise serializers.ValidationError("Invalid label ID format")This way, only expected validation errors are caught, and any unexpected exceptions will bubble up for proper debugging.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def validate_label_id(self, value): | |
| try: | |
| return PyObjectId(value) | |
| except Exception: | |
| raise serializers.ValidationError("Invalid label ID format") | |
| def validate_label_id(self, value): | |
| try: | |
| return PyObjectId(value) | |
| except (ValueError, TypeError): | |
| raise serializers.ValidationError("Invalid label ID format") |
🤖 Prompt for AI Agents
In todo/serializers/add_label_serializer.py around lines 8 to 12, replace the
generic 'except Exception' with 'except ValueError' in the validate_label_id
method to catch only the expected validation errors from PyObjectId. This
ensures that only invalid ObjectId formats raise a ValidationError, while other
unexpected exceptions are not suppressed and can be properly debugged.
| ## API Documentation | ||
| ### Tasks | ||
| #### Add Label to Task |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix markdown formatting issues.
The static analysis correctly identifies several markdown formatting issues that should be addressed for consistency and readability.
Apply these formatting fixes:
ruff check --fix
```
+
+## API Documentation
+
+### Tasks
+
+#### Add Label to Task
+
+```http
-## API Documentation
-### Tasks
-#### Add Label to Task
-```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
102-102: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In README.md around lines 98 to 102, fix the markdown formatting by ensuring
proper spacing and heading structure. Add blank lines before and after the
headings "## API Documentation", "### Tasks", and "#### Add Label to Task" to
improve readability and consistency. Also, wrap the HTTP example or related
content in proper code block syntax using triple backticks to clearly separate
it from the headings.
| { | ||
| "label_id": "456" | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add missing trailing newline.
Files should end with a single newline character as indicated by the static analysis.
Add a trailing newline at the end of the file:
}
> Committable suggestion skipped: line range outside the PR's diff.
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>
124-124: Files should end with a single newline character
null
(MD047, single-trailing-newline)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In README.md at line 124, the file is missing a trailing newline character at
the end. Add a single newline character after the last line to ensure the file
ends properly as required by static analysis tools.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| try: | ||
| task_id = PyObjectId(task_id) | ||
| except Exception: | ||
| return Response( | ||
| data=ApiErrorResponse( | ||
| statusCode=400, | ||
| message="Invalid task ID format", | ||
| errors=[{"detail": "The provided task ID is not valid"}] | ||
| ).model_dump(mode="json"), | ||
| status=status.HTTP_400_BAD_REQUEST | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Use more specific exception handling for ObjectId conversion.
The broad except Exception: clause catches all exceptions, which may mask other potential issues. Consider catching more specific exceptions related to ObjectId conversion.
try:
task_id = PyObjectId(task_id)
- except Exception:
+ except (ValueError, TypeError, InvalidId):
return Response(Note: You may need to import InvalidId from bson.errors if PyObjectId uses BSON ObjectId internally.
Let me verify what specific exceptions PyObjectId raises:
🏁 Script executed:
#!/bin/bash
# Description: Check PyObjectId implementation to understand what exceptions it raises
# Expected: Find the specific exceptions that PyObjectId constructor can raise
ast-grep --pattern $'class PyObjectId {
$$$
def __init__($$$) {
$$$
}
$$$
}'
# Also search for any exception handling patterns in existing PyObjectId usage
rg -A 5 -B 5 "PyObjectId\("Length of output: 1738
🏁 Script executed:
#!/bin/bash
# Display the implementation of PyObjectId to determine which exceptions it raises
cat todo/models/common/pyobjectid.pyLength of output: 430
Use specific exception classes for ObjectId conversion
To avoid masking other errors, catch only the exceptions raised by the ObjectId constructor:
• File: todo/views/task.py
- Add at the top:
from bson.errors import InvalidId
- Change the
exceptblock aroundPyObjectId(task_id)from a broad catch-all to only the known error types:try: task_id = PyObjectId(task_id) -
except Exception:
-
except (InvalidId, TypeError): return Response( data=ApiErrorResponse( statusCode=400, message="Invalid task ID format", errors=[{"detail": "The provided task ID is not valid"}] ).model_dump(mode="json"), status=status.HTTP_400_BAD_REQUEST )
🤖 Prompt for AI Agents
In todo/views/task.py around lines 75 to 85, replace the broad except Exception
clause catching all exceptions during PyObjectId conversion with a more specific
exception handler. Import InvalidId from bson.errors at the top of the file,
then change the except block to catch only InvalidId and any other specific
exceptions PyObjectId might raise. This prevents masking unrelated errors and
improves error handling clarity.
Date: 25 May 2025
Developer Name: Prince Yadav
Issue Ticket Number
#42
Description
Implemented a new RESTful API endpoint to associate an existing public label with a specific task. The implementation follows the project's existing patterns and includes proper validation, error handling, and idempotent operations.
Key changes:
The endpoint:
Example usage:
POST /tasks/{task_id}/labels/
{
"label_id": "label_id_here"
}
Documentation Updated?
Documentation has been updated in the code with proper docstrings and comments.
Under Feature Flag
Database Changes
No database schema changes were required as we're using the existing task and label collections.
Breaking Changes
This is a new endpoint that doesn't affect existing functionality.
Development Tested?
The implementation has been tested locally with various scenarios:
Screenshots
API Response Examples
Success Response (200 OK):
{ "status": "success" }Error Response (404 Not Found):
{ "statusCode": 404, "message": "Task not found", "errors": [ { "detail": "Task with ID {task_id} does not exist" } ] }Test Coverage
Test Coverage Details
The implementation includes:
All tests are passing and maintain the project's existing test coverage standards.
Additional Notes